kvserver: Add a metric for in-progress snapshots #99843
Conversation
|
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
|
This change is |
andrewbaptist
force-pushed
the
23.03.28-delegate-metrics
branch
from
cb9433a to
842d2b8
Compare
andrewbaptist
force-pushed
the
23.03.28-delegate-metrics
branch
from
842d2b8 to
e0a1366
Compare
There was a problem hiding this comment.
Reviewed 2 of 3 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @andrewbaptist)
-- commits line 9 at r2:
range.snapshots.delegate.failures and range.snapshot.delegate.successes had a release note when they were added (4a43cf6). You should add another release note here mentioning the rename, for docs.
pkg/kv/kvserver/replica_command.go line 2859 at r2 (raw file):
) if !selfDelegate { r.store.Metrics().DelegateSnapshotInProgress.Dec(1)
nit: Did you consider moving this up above in the same if block as the inc and deferring the dec?
andrewbaptist
force-pushed
the
23.03.28-delegate-metrics
branch
from
e0a1366 to
bea9903
Compare
There was a problem hiding this comment.
TFTR - I added the release note.
Reviewable status:
Previously, kvoli (Austen) wrote…
range.snapshots.delegate.failuresandrange.snapshot.delegate.successeshad a release note when they were added (4a43cf6). You should add another release note here mentioning the rename, for docs.
Done
pkg/kv/kvserver/replica_command.go line 2859 at r2 (raw file):
Previously, kvoli (Austen) wrote…
nit: Did you consider moving this up above in the same if block as the inc and deferring the dec?
I did consider this, but then it complains about a defer within a loop. I'm not sure which is better actually but I left this for now.
There was a problem hiding this comment.
Reviewed all commit messages.
Reviewable status:
andrewbaptist
force-pushed
the
23.03.28-delegate-metrics
branch
from
bea9903 to
b2831cd
Compare
Fixes: cockroachdb#98242 Knowing how many delegate snapshot requests are currently in-progress will be useful for detecting problems. This change adds a metric for this. It also updates the names of the previous stats to have the prefix `range.snapshots` vs `range.snapshot` to be consistent with other stats. Epic: none Release note (ops change): Adds a new stat range.snapshots.delegate.in-progress and renames two existing stats. They were never part of a release, so better to rename them before 23.1.0 is cut. range.snapshot.delegate.successes -> range.snapshots.delegate.successes range.snapshot.delegate.failures -> range.snapshots.delegate.failures
andrewbaptist
force-pushed
the
23.03.28-delegate-metrics
branch
from
b2831cd to
36ae825
Compare
|
bors r=kvoli |
|
Build failed: |
|
flake here #100119 bors r=kvoli |
|
Build succeeded: |
andrewbaptist
added
the
backport-23.1.x
|
Adding the backport label after the PR has merged does nothing. You need to instruct blathers, as follows: blathers backport 23.1 |
Fixes: #98242
Knowing how many delegate snapshot requests are currently in-progress will be useful for detecting problems. This change adds a metric for this. It also updates the names of the previous stats to have the prefix
range.snapshotsvsrange.snapshotto be consistent with other stats.Epic: none
Release note: None